appDisplay: Get linear index using item instead of page and position

_getLinearPosition() is a function that converts a page and position
index to the "accumulated" index that includes all pages before the
page. The function is used by _addItem() and _moveItem() for getting the
new index of an item inside the _orderedItems array.

Now when passing -1 as position to _addItem() or _moveItem(), this means
the item should be appended to the page. Right now _getLinearPosition()
returns the last item index on the page when passed -1, inserting the
item into _orderedItems at this index will actually not append it, but
insert it between the second last and last item.

To fix it, let's make the whole thing more robust by explicitly passing
an item to _getLinearPosition(). This means we simply no longer have to
assume what "-1" means. Moving the call to _getLinearPosition() to
happen after addItem() and moveItem() ensures that the new item position
is used and not the old one.

This fixes issues where the _orderedItems array gets out of order when
moving or adding items.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2348>

diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index abbadcbed7de6f42258a928eee1e9313159bf285..5e9ad63fe0fbaf75eb90310d4dc9511315abf0b2 100644 (file)
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -1054,20 +1054,18 @@ var BaseAppView = GObject.registerClass({
         return -1;
     }
 
-    _getLinearPosition(page, position) {
+    _getLinearPosition(item) {
+        const [page, position] = this._grid.getItemPosition(item);
+        if (page === -1 || position === -1)
+            throw new Error('Item not found in grid');
+
         let itemIndex = 0;
 
         if (this._grid.nPages > 0) {
-            const realPage = page === -1 ? this._grid.nPages - 1 : page;
-
-            itemIndex = position === -1
-                ? this._grid.getItemsAtPage(realPage).filter(c => c.visible).length - 1
-                : position;
+            for (let i = 0; i < page; i++)
+                itemIndex += this._grid.getItemsAtPage(i).filter(c => c.visible).length;
 
-            for (let i = 0; i < realPage; i++) {
-                const pageItems = this._grid.getItemsAtPage(i).filter(c => c.visible);
-                itemIndex += pageItems.length;
-            }
+            itemIndex += position;
         }
 
         return itemIndex;
@@ -1079,11 +1077,10 @@ var BaseAppView = GObject.registerClass({
         if (this._grid.nPages > 1 && page === -1 && position === -1)
             page = this._findBestPageToAppend();
 
-        const itemIndex = this._getLinearPosition(page, position);
-
-        this._orderedItems.splice(itemIndex, 0, item);
         this._items.set(item.id, item);
         this._grid.addItem(item, page, position);
+
+        this._orderedItems.splice(this._getLinearPosition(item), 0, item);
     }
 
     _removeItem(item) {
@@ -1227,18 +1224,11 @@ var BaseAppView = GObject.registerClass({
     }
 
     _moveItem(item, newPage, newPosition) {
-        const [page, position] = this._grid.getItemPosition(item);
-        if (page === newPage && position === newPosition)
-            return;
+        this._grid.moveItem(item, newPage, newPosition);
 
         // Update the _orderedItems array
-        let index = this._orderedItems.indexOf(item);
-        this._orderedItems.splice(index, 1);
-
-        index = this._getLinearPosition(newPage, newPosition);
-        this._orderedItems.splice(index, 0, item);
-
-        this._grid.moveItem(item, newPage, newPosition);
+        this._orderedItems.splice(this._orderedItems.indexOf(item), 1);
+        this._orderedItems.splice(this._getLinearPosition(item), 0, item);
     }
 
     vfunc_map() {